I don't think this is needed, as mentioned in my other comment - but let me know if this is only intended to be set in cases where the location differs from that of the associated instruction.

Will add comment here to make it clear.

I don't think this should be added as an instruction - only a decorator, just like AssemblyOp.

That said, currently, the AST/assembler doesn't provide a way to attach decorators that don't correspond to an instruction to the AST before assembly.

So - I think the best way to handle this is to support Instruction variants like DebugVar, but do not pretty-print them like instructions, and do not add support in the grammar for parsing them. The compiler will then have an easy way to emit these decorators, but they don't end up getting treated like normal instructions.

I would suggest we add a method on Instruction called has_textual_representation which matches on the instruction variant and returns true if it matches any variant other than those like DebugVar which are not present in the textual format of MASM. I'm happy to bikeshed the name too, but has_textual_representation feels like the most precise way to name it without it unintentionally becoming used for something with different semantics.

I agree!

Isn't this redundant with path_idx, i.e. if we have the path of the source file, then that presumably includes any directory information already, no?

It was intending to kinda match with what other debug info representations do (like DWARF or CodeView), where it tries to keep the base/common root dir shared among other relative paths depending on that root, but since in miden(c) we always operate with full paths, we can simplify this.

If we keep it, I'd make a note that when directory_idx is set, then path_idx is a relative path, otherwise path_idx is expected to be absolute.

We will probably want to explore splitting paths into those components in the future, so we can further reduce the size of the serialized debug info, but I'd treat that as lower priority for now.

I have added a TODO marker for this in 7bbd988, so we can introduce these fields later.

I'd be interested to understand how one can leverage a non-zero scope_depth in practice, i.e. how is it used?

If there is a variable with the same name in a nested/inner block debugger needs to differentiate between that one and the one from outer scope. I will add an example in form of comment here in the code.

I see - in the debugger we only need to care about definitions at the current depth (and above, i.e. lower depth) for purposes of being able to show variable information at the current program point. We don't need to worry about conflicts at the same depth, since only one variable definition at that depth can exist, even if there are multiple scopes with that depth in the current function.

Definitely a good idea to document those semantics here I think, but at least the usefulness is clear to me.

Thanks a lot for the clarification!

If we keep this behavior, I think I would reword this as:

DebugVar is strictly a metadata-carrying decorator used by downstream tooling, i.e. it is non-actionable in the processor itself.

That said, IMO we need some way for the debugger to be notified when variable information is found/changes, so that it can update its internal state accordingly, otherwise we will be forced to step every instruction manually to check for this decorator in case that information changes. I'd prefer not to have to do that.

IMO, we should manifest this either as a raw trace event, modify on_debug to support triggering that callback for this decorator, or add a new callback that is specific to DebugVar. Whichever approach we choose, we'd only invoke the callback when tracing is enabled in the processor.

/cc @bobbinth @plafer

I can't speak to whether this decorator is the right approach for the debugger. Though with #2541 we want to move away from representing AsmOp (and now DebugVar too) as decorators that can be visited needlessly by the processor during execution.

Though for now this is the best we can do, provided that the DebugVar decorator is the right approach for the debugger.

Note: is this a feature that would only be accessible for programs written in Rust, or also from MASM? Or in other words, would there be fewer features made available by the debugger for debugging programs written in MASM compared to programs written in Rust?

Note: is this a feature that would only be accessible for programs written in Rust, or also from MASM? Or in other words, would there be fewer features made available by the debugger for debugging programs written in MASM compared to programs written in Rust?

If there is a way of using a concept of "variable" at MASM level we can create a decorator for it -- since this is not Rust specific, but rather an abstraction. However, it needs to have a name, type..., so debugger can decode its value properly.

@plafer Since MASM doesn't have variables, this decorator wouldn't ever be produced from Miden Assembly sources. The closest we have are procedure locals, but those aren't variables in any practical sense; rather they collectively represent a fixed-size region of automatically-allocated, procedure-scoped memory, so I don't think it would make sense to use DebugVar for them - at least not without some MASM syntax for local variables that gives them a name and type, and is then used to derive the size and placement of procedure locals needed for that procedure.

But, as @djolertrk noted, this can be used for any source language with variables, it isn't Rust-specific.

On the topic of debug decorators like this and AssemblyOp, I'd be happy to replace them with something that doesn't rely on the decorator mechanism at all, but I'm not actually sure what that would be. All we really need is a way to map a MAST node to the debug information that applies to it, ideally with a way to disambiguate the debug information based on context (e.g. current call stack). AFAIK, that's essentially how decorators work now though (sans the disambiguation bit), so it isn't clear to me what we gain or lose by adding decorators like this. Definitely open to figuring out how best to handle this sort of thing going forward though.

With #2541 we basically already have a plan to move out any current "decorator" which only "provides metadata" (TLDR they get their own separate CSR matrix). But since that doesn't exist yet, we can just add the DebugVar as a decorator for now (as-is in this PR), and we'll pull it out along with AsmOp when we address #2541.